feat(users): Modify users module to implement style guidelines. #1208
Conversation
|
Can you please fix the tests. |
itelo
force-pushed
the
users-styleguide
branch
5 times, most recently
from
a4870ba to
ee13207
Compare
|
@ilanbiala @codydaig one of you to review it? a big client-side PR so it's best to merge soon before we're having conflicts there from other merged PRs |
|
Someone is looking to this PR? |
|
Yes. Rebase, and I'll start reviewing & testing. |
|
@mleanos check now (: |
| @@ -2,13 +2,13 @@ | |||
| <div class="page-header"> | |||
| <div class="row"> | |||
| <div class="col-md-6"> | |||
| <h1 ng-bind="user.username"></h1> | |||
| <h1 ng-bind="vm.user.username"></h1> | |||
| </div> | |||
| <div class="col-md-4"> | |||
| <a class="btn btn-primary" ui-sref="admin.user-edit({userId: user._id})"> | |||
There was a problem hiding this comment.
missing vm should be {userId: vm.user._id} for the state param.
|
@itelo I pulled this down & tested. Didn't notice any issues, other than the missing Overall, this looks really good. Awesome job! Can someone else do a review here? Quite a bit of code, so I could have missed something. |
|
@mleanos i re-checked everything and discover bugs, i already fixed them... |
|
@meanjs/contributors More eyes on this? I haven't tested since the issues were taken care of, but I plan to in the next couple of days. |
| @@ -26,7 +26,7 @@ | |||
| function compileDirective(template) { | |||
| // function to compile a fresh directive with the given template, or a default one | |||
| // input form with directive | |||
| if (!template) template = '<input type="password" id="newPassword" name="newPassword" class="form-control" ng-model="passwordMock.newPassword" placeholder="New Password" autocomplete="new-password" uib-popover="{{popoverMsg}}" uib-popover-trigger="focus" uib-popover-placement="top" password-validator required>' + | |||
| if (!template) template = '<input type="password" id="newPassword" name="newPassword" class="form-control" ng-model="passwordMock.newPassword" placeholder="New Password" autocomplete="new-password" uib-popover="{{getPopoverMsg}}" uib-popover-trigger="focus" uib-popover-placement="top" password-validator required>' + | |||
There was a problem hiding this comment.
I don't think vm is relevant here. We're only testing the directive, and there's no context for a controller here. IIRC, from working with these tests, I think this is the correct way to write isolated tests for directives.
|
LGTM. |
|
I just pulled this down again, and tested. Everything seems good to me. Didn't notice any issues. I think this is ready. I'll try to merge soon, pending one or two more LGTM's. |
|
Yep, you can merge. |
feat(users): Modify users module to implement style guidelines.
|
@itelo This is a huge huge help. Thanks! It's nice to have the style-guide here now :) |
No description provided.